Skip to content

Conversation

@jarrodu
Copy link
Member

@jarrodu jarrodu commented Nov 20, 2016

There is a lot in this pull request and I think it may be a challenge to review. It maybe easiest to review the commits one at a time. The hardest will be the _layouts commit.

Everything builds locally and no broken links are introduces.

The main motivation for doing all of this is to make the site easier to understand and modify. It is pretty much just a large refactor with a few design changes to simplify things.

_config.yml Outdated
events/README.md,
Gemfile,
Gemfile.lock
]
Copy link
Member

@ashawley ashawley Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even simpler:

exclude:
  - bundle-vendor/
  - "*.md"
  - Gemfile
  - Gemfile.lock

Copy link
Member Author

@jarrodu jarrodu Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*.md knocks out nearly the whole site. I get your point though. Your version is cleaner.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*.md knocks out nearly the whole site.

Sorry, it was untested.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. I will update the config file with your improved style.

download/all.md Outdated
{% if releaseVersion == possibleVersion %}
<div>
<a href="{{ page.url }}">{{ page.title }}</a>
</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This HTML needs to start in the first column, instead it's being shown as source code:

<div>
  <a href="/download/2.12.0.html">Scala 2.12.0</a>
</div>
  
  
  
<div>
  <a href="/download/2.12.0-RC2.html">Scala 2.12.0-RC2</a>
</div>
  
...

Copy link
Member Author

@jarrodu jarrodu Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. I will fix it. 👍

@SethTisue
Copy link
Member

Are we sure that _tools/redirections.txt is unused? Maybe @fsalvi uses it?

@jarrodu
Copy link
Member Author

jarrodu commented Nov 21, 2016

I am not sure if it is still in use. I was going on the last modification date and taking a guess. At least it does not seem to be needed by Jekyll.

@SethTisue
Copy link
Member

I don't know the templating stuff well enough to give a good technical review on that. But, other than my question about redirections.txt (hopefully Fabien will respond), LGTM.

@SethTisue
Copy link
Member

Oh, now when I building I see:

[2016-11-21 08:38:52] ERROR `/webscripts/ajax/getFromTwitter.php' not found.

I don't recall seeing that error before.

@jarrodu
Copy link
Member Author

jarrodu commented Nov 21, 2016

We can always role back a few commits if need be if the templates break something.

I think the /webscripts/ajax/... and a few other things are sitting on the production server. It would be great to get everything in one place. The last few steps from building the site and getting to production are still a bit mysterious to me. I am interested in learning more though.

@fsalvi
Copy link
Contributor

fsalvi commented Nov 21, 2016

About the building error, I guess it's because of the last modification I made:
3657212

I just fixed it.

@fsalvi
Copy link
Contributor

fsalvi commented Nov 21, 2016

I couldn't see any script using _tools/redirections.txt to automatically update .htaccess
I guess it was used at first to not forget some important redirections... But afaik, it's not used anymore.

@fsalvi
Copy link
Contributor

fsalvi commented Nov 21, 2016

getFromTwitter.php is in a private repository on github because it contains keys and secrets for the API calls

@jarrodu jarrodu changed the title Spring cleaning WIP Spring cleaning Nov 21, 2016
@jarrodu jarrodu self-assigned this Nov 21, 2016
@jarrodu
Copy link
Member Author

jarrodu commented Nov 22, 2016

I rebased onto master. I guess I still have some things to learn. A ton of commits were added to this PR. The only changes I made are the last two.

@jarrodu
Copy link
Member Author

jarrodu commented Nov 22, 2016

I am not sure what to do. It looks like merging this in may put a lot of redundant commits into the commit log. Does anyone know how to deal this this situation? I will do some more research and try to fix it.

@jarrodu jarrodu mentioned this pull request Nov 22, 2016
@jarrodu
Copy link
Member Author

jarrodu commented Nov 22, 2016

Okay I think the easiest thing to do was to just make a new branch and get rid of the rebase and the weird merge.

See #550

@jarrodu jarrodu closed this Nov 22, 2016
@ashawley
Copy link
Member

ashawley commented Nov 22, 2016

Looks like your new commits are at the top, so it should be easy. Something like this could fix it (where the remote jarrodwb for me is likely origin for you):

# original commit was 34f2333, but use the 3 commits from the diverged branch:
git rebase --onto 34f2333 spring-cleaning~3 spring-cleaning
git checkout master
git pull upstream master
git checkout -
git rebase master
git diff jarrodwb/spring-cleaning
git push -f jarrodwb spring-cleaning

There's no output on the last line, so you can effectively push up changes and know you are only fixing the version history but nothing else.

@jarrodu
Copy link
Member Author

jarrodu commented Nov 22, 2016

Thanks Aaron for the information. It looks like that may work. I already moved things over to #550.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants